Fix stability of Definition entities#1253
Open
lionel- wants to merge 14 commits into
Open
Conversation
lionel-
force-pushed
the
oak/salsa-16-document-wire-url
branch
from
f6332d0 to
14bacca
Compare
lionel-
force-pushed
the
oak/salsa-17-interning
branch
from
59a2813 to
0cd9628
Compare
DavisVaughan
approved these changes
Jun 9, 2026
| pub(crate) fn definition( | ||
| self, | ||
| db: &'db dyn Db, | ||
| scope: ScopeId, |
Contributor
There was a problem hiding this comment.
Suggested change
| scope: ScopeId, | |
| scope_id: ScopeId, |
consistency with def_id and other usages
Contributor
There was a problem hiding this comment.
should pass through to DefinitionSite too i think
|
|
||
| /// Every `Definition` in a file, keyed by its definition site. | ||
| #[derive(Debug, PartialEq, Eq, salsa::Update)] | ||
| struct FileDefinitions<'db> { |
Contributor
There was a problem hiding this comment.
Suggested change
| struct FileDefinitions<'db> { | |
| struct Definitions<'db> { |
would also be happy with this since the method is definitions()
Only canonicalise when comparing against paths from external sources (e.g. normalised by R). Canonicalisation may corrupt the identity of paths sent by frontends, and may prevent matching files at all when they are deleted (at which point a non-canonicalised path sent by the frontend can't be matched against canonicalised paths we've stored as keys)
lionel-
force-pushed
the
oak/salsa-16-document-wire-url
branch
from
14bacca to
7743983
Compare
lionel-
force-pushed
the
oak/salsa-17-interning
branch
from
0cd9628 to
25c9de8
Compare
lionel-
force-pushed
the
oak/salsa-16-document-wire-url
branch
from
7743983 to
a9b0bf2
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Branched from #1252
Progress towards #1212
Like in ty, our
Definitions are a tracked struct with some degree of stability across edits. Unlike in ty, the definitions are not created within the semantic index, they are created outside so that we keep the index salsa-free, which allows it to be easily reused to figure out data flow dependencies in non-salsa contexts.Because the definitions are created outside the index we need to be a bit careful. This is a tracked struct, not an interned one, and that means that different Salsa queries create different instances of the definition that don't compare equal, even if they are the same definition. (See https://hackmd.io/5ddaZLKYSVC_YTTD3SzRDw?view#Tracked-Structs, thanks to @DavisVaughan for finding this doc.)
To ensure definitions have a single identity, we now funnel all definitions lookup through a single
File::definitions()query that creates a map ofDefinitionkeyed byScopeandDefId(bundled in a new structDefinitionSite).New tests ensure that definitions are not invalidated when they shift in a file or when definitions are inserted before (which changes the lookup key but not the definition ID, so downstream queries remain valid even if their key is now outdated).